-
Notifications
You must be signed in to change notification settings - Fork 0
Adding scheduling decision CRD: "why/why" not api #307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
internal/scheduler/nova/pipeline.go
Outdated
|
||
if err := c.Client.Update(context.Background(), &existing); err != nil { | ||
// Check if it's a conflict error (concurrent update) | ||
if attempt < maxRetries-1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this happen? I would have thought that the Update
method is threadsafe and consistent with the state in kubernetes. Otherwise we should add a mutex, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CRD schema treats spec and status as separated update, so should be fine if there is a single entity changing the spec. Simplified for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for checking that up!
Test Coverage ReportCoverage in main module (internal/): 73.3%
Coverage in reservations module (reservations/internal/): 74.6%
|
This contribution adds an api to cortex that describes why a virtual machine has been placed on a specific compute host. It exposes the raw pipeline weights with a kubernetes custom resource, and the associated operator calculates a human-readable description from the provided values. The resource is also connected to the nova visualizer so that decisions can be spectated in the browser removing this dependency on the mqtt broker.